Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Comment Template Block: Add test coverage for context setting #50879

Merged
merged 7 commits into from
May 30, 2023
Merged

Comment Template Block: Add test coverage for context setting #50879

merged 7 commits into from
May 30, 2023

Conversation

ockham
Copy link
Contributor

@ockham ockham commented May 23, 2023 •
edited
Loading

What?

Add a unit test to verify that the Comment Template block sets commentId context as expected. Follow up to #50279; see #50279 (comment) for context.

Why?

Conversation on #50279 indicates that it's fairly easy to make a change to the relevant code which would break this behavior. Consequently, it makes sense to guard against such breakage.

How?

By inserting a block which requires commentId context to work via the render_block hook.

Testing Instructions

  1. Verify that test pass. Locally, this can be done via npm run test:unit:php -- --group=blocks.
  2. Now revert 4409ba4 and rebuild blocks (npm run build). Re-run the test, and verify that it fails now.

Note

In the long run, this test should be merged into wordpress-develop's renderCommentTemplate.php test file.

@ockham ockham added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Block] Comment Template Affects the Comment Template Block labels May 23, 2023
@ockham ockham self-assigned this May 23, 2023
@github-actions
Copy link

Flaky tests detected in e4c1ff7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5058441320
📝 Reported issues:

@ockham ockham requested a review from gziolo May 23, 2023 15:15
@ockham ockham marked this pull request as ready for review May 23, 2023 15:15
@ockham ockham mentioned this pull request May 23, 2023
Merged
@ockham ockham force-pushed the add/test-coverage-for-comments-template-block-context branch from d09ba37 to 4ab65a5 Compare May 24, 2023 14:05
This was referenced May 25, 2023
Merged
Closed
@ockham
Copy link
Contributor Author

ockham commented May 25, 2023

In the long run, this test should be merged into wordpress-develop's renderCommentTemplate.php test file.

PR: WordPress/wordpress-develop#4507

gziolo
gziolo approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this test covering special case for the Comment Template block. It was very helpful to see the PR targeting WordPress Core to confrim that setup code is only duplicated to run it in the Gutenberg repo.

Have you considered adding an additional assertion to verify whether the $comment_author_name_block variable is non-empty string or has some shape at all. It would cover the case when the block would stop rendering any output, but it might be covered with other tests. So it isn't a blocker, but something for you to decide.

@ockham
Copy link
Contributor Author

ockham commented May 30, 2023

Have you considered adding an additional assertion to verify whether the $comment_author_name_block variable is non-empty string or has some shape at all. It would cover the case when the block would stop rendering any output, but it might be covered with other tests. So it isn't a blocker, but something for you to decide.

Ah yeah, sounds like a great idea! I'll add something.

@ockham ockham merged commit baf2251 into trunk May 30, 2023
@ockham ockham deleted the add/test-coverage-for-comments-template-block-context branch May 30, 2023 13:19
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone May 30, 2023
@ockham ockham mentioned this pull request May 31, 2023
Closed
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
@ockham @sethrubenstein
Comment Template Block: Add test coverage for context setting (WordPr…
74987d5 
…ess#50879)

Add a unit test to verify that the Comment Template block sets `commentId` context as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

Assignees

@ockham ockham

Labels
[Block] Comment Template Affects the Comment Template Block [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Milestone
Gutenberg 16.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ockham @gziolo